Skip to content

Conversation

@networkfusion
Copy link
Member

@networkfusion networkfusion commented Jul 7, 2025

Description

  • Update docker/build-push-action to V6
  • Update actions/checkout to V6

Motivation and Context

Keeps GH actions up-to-date

How Has This Been Tested?

CI

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dev Containers (changes related with Dev Containers, has no impact on code or features)
  • Dependencies/declarations (update dependencies or assembly declarations and changes associated, has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

Summary by CodeRabbit

  • Chores
    • CI workflows upgraded to newer GitHub Actions versions for improved reliability and support.
    • Container publishing now emits both a version-specific tag and a "latest" tag to simplify image consumption.
    • Tag specification formatting updated; no functional or user-facing behavior changed.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 7, 2025

Walkthrough

Updated devcontainer GitHub Actions: bumped actions/checkout from v4 → v5 across workflows and upgraded docker/build-push-action from v5 → v6; several workflows now supply two image tags (versioned and :latest) in the build-and-push step. No other control-flow changes.

Changes

Cohort / File(s) Change Summary
Devcontainer workflows
.github/workflows/devcontainer-all.yaml, .github/workflows/devcontainer-azurertos.yaml, .github/workflows/devcontainer-chibios.yaml, .github/workflows/devcontainer-esp32.yml, .github/workflows/devcontainer-freertos-nxp.yaml, .github/workflows/devcontainer-ti.yaml, .github/workflows/devcontainer-smoketest.yaml
Upgraded actions/checkout from v4v5 in all workflows; upgraded docker/build-push-action from v5v6 where used. In the Build and Push step some workflows now pass two tags (a versioned tag using ${GCR_VERSION} or equivalent and :latest). No other logic or error handling changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant GH as GitHub Actions
    participant Checkout as actions/checkout@v5
    participant BuildPush as docker/build-push-action@v6
    participant Registry as Container Registry

    GH->>Checkout: checkout repository (actions/checkout@v5)
    Checkout-->>GH: source checked out
    GH->>BuildPush: build + push (context, file, push, tags)
    note right of BuildPush `#F3F8FF`: tags input now includes\nversioned tag and `:latest` where configured
    BuildPush->>Registry: push image :${GCR_VERSION}
    BuildPush->>Registry: push image :latest
    Registry-->>BuildPush: push acknowledged
    BuildPush-->>GH: step completed
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify tags formatting and interpolation in workflows that added tags.
  • Confirm actions/checkout@v5 compatibility with any workflow-specific inputs (if present).
  • Spot-check docker/build-push-action@v6 usage to ensure required inputs (context, file, push) are unchanged.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title '[CI] Update github actions' clearly and concisely summarizes the main change—upgrading GitHub Actions versions across multiple CI workflows.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-docker-build-push-action

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4325845 and 1ad251a.

📒 Files selected for processing (6)
  • .github/workflows/devcontainer-all.yaml (2 hunks)
  • .github/workflows/devcontainer-azurertos.yaml (2 hunks)
  • .github/workflows/devcontainer-chibios.yaml (2 hunks)
  • .github/workflows/devcontainer-esp32.yml (2 hunks)
  • .github/workflows/devcontainer-freertos-nxp.yaml (2 hunks)
  • .github/workflows/devcontainer-ti.yaml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
  • .github/workflows/devcontainer-freertos-nxp.yaml
  • .github/workflows/devcontainer-azurertos.yaml
  • .github/workflows/devcontainer-chibios.yaml
  • .github/workflows/devcontainer-ti.yaml
  • .github/workflows/devcontainer-all.yaml
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: networkfusion
Repo: nanoframework/nf-interpreter PR: 3198
File: .github/workflows/devcontainer-esp32.yml:62-64
Timestamp: 2025-11-05T16:43:42.592Z
Learning: In the nanoframework/nf-interpreter repository, the team prefers YAML folded scalar (>) or pipe (|) format for docker/build-push-action tags input over single-line comma-separated format for better readability in GitHub Actions workflows.
📚 Learning: 2025-11-05T16:43:42.592Z
Learnt from: networkfusion
Repo: nanoframework/nf-interpreter PR: 3198
File: .github/workflows/devcontainer-esp32.yml:62-64
Timestamp: 2025-11-05T16:43:42.592Z
Learning: In the nanoframework/nf-interpreter repository, the team prefers YAML folded scalar (>) or pipe (|) format for docker/build-push-action tags input over single-line comma-separated format for better readability in GitHub Actions workflows.

Applied to files:

  • .github/workflows/devcontainer-esp32.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
  • GitHub Check: nf-interpreter (Check_Build_Options)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, All, 915)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, All)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, TI, 915)
  • GitHub Check: build-target (ESP32_C3, Debug, ESP32)
  • GitHub Check: build-target (ESP32_H2_THREAD, Debug, ESP32)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, FreeRTOS-NXP)
  • GitHub Check: build-target (M5Core2, Debug, ESP32)
  • GitHub Check: build-target (SL_STK3701A, Debug, All)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, ChibiOS)
  • GitHub Check: build-target (ESP32_C6_THREAD, Debug, ESP32)
  • GitHub Check: build-target (ESP_WROVER_KIT, Debug, ESP32)
  • GitHub Check: build-target (ESP32_S3, Debug, ESP32)
  • GitHub Check: build-target (ESP32_S2_USB, Debug, ESP32)
  • GitHub Check: build-target (SL_STK3701A, Debug, AzureRTOS)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, All)
🔇 Additional comments (2)
.github/workflows/devcontainer-esp32.yml (2)

58-64: Version upgrade and tags format look correct.

The upgrade to docker/build-push-action@v6 aligns with PR objectives, and the tags format (pipe | with two image tags) is both compatible and consistent with your team's preferred readability style. Based on learnings, this follows the established convention for this repository.


29-29: Reject review comment: false premise about PR objectives.

The original review comment claims PR objectives state "upgrading actions/checkout to v6", but the actual git commit history shows Update actions/checkout to V5. Line 29 correctly shows actions/checkout@v5, which aligns with the stated objective. The version upgrade has been properly implemented. Additionally, docker/build-push-action@v6 at line 58 matches its stated upgrade objective. No mismatch exists.

Likely an incorrect or invalid review comment.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (6)
.github/workflows/devcontainer-all.yaml (1)

55-63: Confirm v6 input/output parity & pin digest for reproducibility

docker/build-push-action v6 removes some deprecated inputs and introduces new defaults (e.g., load: true auto-loads images). The current with: block (file, push, tags) is still supported, but please double-check the v6 release notes to ensure no silent behaviour changes affect this workflow.
For supply-chain stability, consider pinning to the major tag + digest, e.g. docker/build-push-action@v6@sha256:<digest>.

.github/workflows/devcontainer-ti.yaml (1)

55-63: Same v6 compatibility & digest pinning considerations

Replicate the v6 release-notes check and digest pinning suggestion here to maintain consistent, reproducible builds across all dev-container workflows.

.github/workflows/devcontainer-azurertos.yaml (1)

55-63: Validate behaviour change after upgrading to v6

Ensure push: true and tag handling behave exactly as before; v6 tweaks cache-related defaults that could impact build time or registry content. Pin to a digest for immutability if possible.

.github/workflows/devcontainer-esp32.yml (1)

55-63: Upgrade sanity-check and optional digest pin

Confirm no deprecated inputs (e.g., context, platforms) are implicitly relied upon. Consider locking the action to @v6@sha256:<digest> to avoid unexpected future changes.

.github/workflows/devcontainer-chibios.yaml (1)

55-63: Consistency check after bump to v6

Double-check the ChibiOS image still builds & pushes as expected with the new major version and apply digest pinning for deterministic CI runs.

.github/workflows/devcontainer-freertos-nxp.yaml (1)

55-57: Verify v6 migration settings (provenance, sbom, cache) before merging

docker/build-push-action@v6 introduces new defaults (provenance: true, sbom: true, different cache-from/to syntax).
If you don’t explicitly want provenance/SBOM generation or intend to keep the current cache behaviour, add the relevant inputs:

-    - name: Build and Push Docker Image
-      uses: docker/build-push-action@v6
+    - name: Build and Push Docker Image
+      uses: docker/build-push-action@v6
+      with:
+        provenance: false   # disable SLSA attestation if not required
+        sbom: false         # disable SBOM upload if not required
+        # cache-from / cache-to examples:
+        # cache-from: type=registry,ref=${{ env.CONTAINER_REPO }}/...:cache
+        # cache-to:   type=registry,ref=${{ env.CONTAINER_REPO }}/...:cache,mode=max

(Or keep them enabled deliberately—just make the choice explicit.)

Also consider pinning to a full semver tag or SHA for supply-chain safety, e.g. docker/[email protected].

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e8b977d and 1af8585.

📒 Files selected for processing (6)
  • .github/workflows/devcontainer-all.yaml (1 hunks)
  • .github/workflows/devcontainer-azurertos.yaml (1 hunks)
  • .github/workflows/devcontainer-chibios.yaml (1 hunks)
  • .github/workflows/devcontainer-esp32.yml (1 hunks)
  • .github/workflows/devcontainer-freertos-nxp.yaml (1 hunks)
  • .github/workflows/devcontainer-ti.yaml (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
  • GitHub Check: nf-interpreter (Check_Build_Options)
  • GitHub Check: build-target (ESP32_C3, Debug, ESP32)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, All, 915)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, TI, 915)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, All)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, FreeRTOS-NXP)
  • GitHub Check: build-target (ESP32_H2_THREAD, Debug, ESP32)
  • GitHub Check: build-target (M5Core2, Debug, ESP32)
  • GitHub Check: build-target (ESP32_S2_USB, Debug, ESP32)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, ChibiOS)
  • GitHub Check: build-target (ESP32_C6_THREAD, Debug, ESP32)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, All)
  • GitHub Check: build-target (ESP32_S3, Debug, ESP32)
  • GitHub Check: build-target (ESP_WROVER_KIT, Debug, ESP32)
  • GitHub Check: build-target (SL_STK3701A, Debug, All)
  • GitHub Check: build-target (SL_STK3701A, Debug, AzureRTOS)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
.github/workflows/devcontainer-esp32.yml (1)

58-65: Optional: enable SBOM/provenance attestations with v6.

Adds supply‑chain metadata with minimal overhead.
Apply within this block:

       with:
         file: ${{ env.CONTAINER_SRC_FILE }}
         push: true # Will only build if this is not here
         tags: |
           ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }}
           ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
+        sbom: true
+        provenance: true

Docs: SBOM/provenance on build-push-action@v6; v6 also emits a job summary by default. (docs.docker.com)

.github/workflows/devcontainer-azurertos.yaml (2)

58-65: Optional: add SBOM/provenance generation.

Strengthens image traceability; simple toggle with v6.

       with:
         file: ${{ env.CONTAINER_SRC_FILE }}
         push: true # Will only build if this is not here
         tags: |
           ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }}
           ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
+        sbom: true
+        provenance: true

See guidance. (docs.docker.com)


58-65: Heads-up: v6 build record artifact.

Same note as the ESP32 workflow—ensure any actions/download-artifact steps don’t unintentionally fetch “*.dockerbuild” artifacts.

Use the audit script from the previous comment; it covers all workflows. (github.com)

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1af8585 and 051a2bd.

📒 Files selected for processing (6)
  • .github/workflows/devcontainer-all.yaml (1 hunks)
  • .github/workflows/devcontainer-azurertos.yaml (1 hunks)
  • .github/workflows/devcontainer-chibios.yaml (1 hunks)
  • .github/workflows/devcontainer-esp32.yml (1 hunks)
  • .github/workflows/devcontainer-freertos-nxp.yaml (1 hunks)
  • .github/workflows/devcontainer-ti.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • .github/workflows/devcontainer-chibios.yaml
  • .github/workflows/devcontainer-freertos-nxp.yaml
  • .github/workflows/devcontainer-ti.yaml
  • .github/workflows/devcontainer-all.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
  • GitHub Check: nf-interpreter (Check_Build_Options)
  • GitHub Check: build-target (ESP32_C6_THREAD, Debug, ESP32)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, All)
  • GitHub Check: build-target (SL_STK3701A, Debug, AzureRTOS)
  • GitHub Check: build-target (ESP32_C3, Debug, ESP32)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, FreeRTOS-NXP)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, All, 915)
  • GitHub Check: build-target (ESP32_H2_THREAD, Debug, ESP32)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, TI, 915)
  • GitHub Check: build-target (ESP_WROVER_KIT, Debug, ESP32)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, ChibiOS)
  • GitHub Check: build-target (ESP32_S3, Debug, ESP32)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, All)
  • GitHub Check: build-target (ESP32_S2_USB, Debug, ESP32)
  • GitHub Check: build-target (M5Core2, Debug, ESP32)
  • GitHub Check: build-target (SL_STK3701A, Debug, All)
🔇 Additional comments (3)
.github/workflows/devcontainer-esp32.yml (2)

58-58: Upgrade to docker/build-push-action@v6 looks good.

Inputs used here (file, push, tags) are compatible with v6; no breaking changes for this step.
See: docker/build-push-action docs. (github.com)


58-65: Heads-up: docker/build-push-action@v6 uploads build-record artifacts — no action required now.

  • v6 present in: .github/workflows/devcontainer-esp32.yml, .github/workflows/devcontainer-all.yaml, .github/workflows/devcontainer-azurertos.yaml, .github/workflows/devcontainer-chibios.yaml, .github/workflows/devcontainer-freertos-nxp.yaml, .github/workflows/devcontainer-ti.yaml
  • No occurrences of actions/download-artifact were found in .github/workflows. To ignore build-record artifacts in any download-artifact steps use:
    with:
    pattern: "!*.dockerbuild"
.github/workflows/devcontainer-azurertos.yaml (1)

58-58: Upgrade to docker/build-push-action@v6 looks good.

Inputs here remain valid with v6; behavior is unchanged for build/push/tags.
Docs reference. (github.com)

@networkfusion networkfusion changed the title Update docker/build-push-action Update github actions Nov 5, 2025
@networkfusion networkfusion changed the title Update github actions [CI] Update github actions Nov 5, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 051a2bd and bdbde28.

📒 Files selected for processing (7)
  • .github/workflows/devcontainer-all.yaml (2 hunks)
  • .github/workflows/devcontainer-azurertos.yaml (2 hunks)
  • .github/workflows/devcontainer-chibios.yaml (2 hunks)
  • .github/workflows/devcontainer-esp32.yml (2 hunks)
  • .github/workflows/devcontainer-freertos-nxp.yaml (2 hunks)
  • .github/workflows/devcontainer-smoketest.yaml (1 hunks)
  • .github/workflows/devcontainer-ti.yaml (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/devcontainer-smoketest.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
  • .github/workflows/devcontainer-ti.yaml
  • .github/workflows/devcontainer-all.yaml
  • .github/workflows/devcontainer-chibios.yaml
  • .github/workflows/devcontainer-azurertos.yaml
  • .github/workflows/devcontainer-freertos-nxp.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
  • GitHub Check: nf-interpreter (Check_Build_Options)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, All, 915)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, TI, 915)
  • GitHub Check: build-target (SL_STK3701A, Debug, All)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, All)
  • GitHub Check: build-target (ESP32_C6_THREAD, Debug, ESP32)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, FreeRTOS-NXP)
  • GitHub Check: build-target (ESP32_H2_THREAD, Debug, ESP32)
  • GitHub Check: build-target (SL_STK3701A, Debug, AzureRTOS)
  • GitHub Check: build-target (ESP32_S2_USB, Debug, ESP32)
  • GitHub Check: build-target (ESP_WROVER_KIT, Debug, ESP32)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, All)
  • GitHub Check: build-target (ESP32_C3, Debug, ESP32)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, ChibiOS)
  • GitHub Check: build-target (M5Core2, Debug, ESP32)
  • GitHub Check: build-target (ESP32_S3, Debug, ESP32)
🔇 Additional comments (1)
.github/workflows/devcontainer-esp32.yml (1)

62-64: Multi-tag push strategy is idiomatic. The workflow now pushes both a versioned tag (${{ env.GCR_VERSION }}) and latest, which is a standard pattern for container images and maintains backward compatibility for consumers relying on the latest tag.

All referenced environment variables are properly defined (lines 6–9) and the GCR_VERSION is populated dynamically in the prior step (lines 39–45).

Suggested by code-rabbit.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
.github/workflows/devcontainer-chibios.yaml (1)

62-64: Clarify tags format: prefer explicit single-line CSV over YAML folded scalar.

The current YAML > (folded scalar) converts the multi-line block into a single line with spaces: "ghcr.io/owner/name:version, ghcr.io/owner/name:latest". While this produces valid CSV, the non-standard presentation and reliance on YAML's implicit folding behavior makes it less clear and potentially fragile.

Recommendation: Replace the folded scalar with an explicit single-line CSV string for clarity and consistency with standard GitHub Actions patterns.

-        tags: >
-          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},
-          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
+        tags: ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest

Alternatively, for readability on long lines, introduce an intermediate step to build the CSV string in a env: block or as a separate step output.

.github/workflows/devcontainer-ti.yaml (1)

62-64: Tags format consistency issue across workflows.

Same YAML folded scalar pattern as .github/workflows/devcontainer-chibios.yaml. See note there regarding clarification to explicit single-line CSV.

-        tags: >
-          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},
-          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
+        tags: ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
.github/workflows/devcontainer-azurertos.yaml (1)

62-64: Tags format: consistent with other workflow changes in this PR.

Recommend refactoring to explicit single-line CSV (see .github/workflows/devcontainer-chibios.yaml for details).

-        tags: >
-          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},
-          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
+        tags: ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bdbde28 and 4325845.

📒 Files selected for processing (6)
  • .github/workflows/devcontainer-all.yaml (2 hunks)
  • .github/workflows/devcontainer-azurertos.yaml (2 hunks)
  • .github/workflows/devcontainer-chibios.yaml (2 hunks)
  • .github/workflows/devcontainer-esp32.yml (2 hunks)
  • .github/workflows/devcontainer-freertos-nxp.yaml (2 hunks)
  • .github/workflows/devcontainer-ti.yaml (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • .github/workflows/devcontainer-freertos-nxp.yaml
  • .github/workflows/devcontainer-all.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: nf-interpreter (Nightly build) (Check_Build_Options)
  • GitHub Check: nf-interpreter (Check_Build_Options)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, All, 915)
  • GitHub Check: build-target (TI_CC1352R1_LAUNCHXL, Debug, TI, 915)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, All)
  • GitHub Check: build-target (ESP32_C3, Debug, ESP32)
  • GitHub Check: build-target (NXP_MIMXRT1060_EVK, Debug, FreeRTOS-NXP)
  • GitHub Check: build-target (ESP32_H2_THREAD, Debug, ESP32)
  • GitHub Check: build-target (ESP32_S2_USB, Debug, ESP32)
  • GitHub Check: build-target (ESP32_S3, Debug, ESP32)
  • GitHub Check: build-target (ESP32_C6_THREAD, Debug, ESP32)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, All)
  • GitHub Check: build-target (ESP_WROVER_KIT, Debug, ESP32)
  • GitHub Check: build-target (M5Core2, Debug, ESP32)
  • GitHub Check: build-target (SL_STK3701A, Debug, All)
  • GitHub Check: build-target (SL_STK3701A, Debug, AzureRTOS)
  • GitHub Check: build-target (ST_STM32F769I_DISCOVERY, Debug, ChibiOS)
🔇 Additional comments (8)
.github/workflows/devcontainer-chibios.yaml (2)

29-29: Action version upgrade looks good.

Upgrading actions/checkout to v5 is appropriate. Verify compatibility notes if any custom behaviors relied on v4-specific features.


58-58: Action version upgrade compatible with current tags format.

Upgrading docker/build-push-action to v6 is approved. Tags in docker/build-push-action v6 expects comma-delimited list of tags.

.github/workflows/devcontainer-ti.yaml (2)

29-29: Action version upgrade approved.


58-58: Action version upgrade approved.

.github/workflows/devcontainer-azurertos.yaml (2)

29-29: Action version upgrade approved.


58-58: Action version upgrade approved.

.github/workflows/devcontainer-esp32.yml (2)

29-29: Action version upgrade approved.


58-58: Action version upgrade approved; address prior concern about tags format compatibility.

A previous review raised a critical issue about docker/build-push-action@v6 tags format compatibility. The current code uses YAML > (folded scalar) which technically produces CSV output, but this presentation is non-standard and may have been the source of confusion in the prior review.

Comment on lines 62 to 64
tags: >
${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},
${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Resolve prior critical flag by refactoring to explicit single-line CSV.

A previous review flagged this tags format as incompatible with docker/build-push-action@v6, requiring CSV format. Docker/build-push-action expects a comma-delimited list of tags. The current YAML > (folded scalar) does produce a single-line CSV string, but using the standard explicit format will eliminate ambiguity and definitively resolve the prior concern.

-        tags: >
-          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},
-          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
+        tags: ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
tags: >
${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},
${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
tags: ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }},${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest
🤖 Prompt for AI Agents
.github/workflows/devcontainer-esp32.yml around lines 62 to 64: the tags value
is written as a folded scalar (">") which can be ambiguous for
docker/build-push-action@v6; replace it with an explicit single-line
comma-separated list of tags so the action receives a clear CSV (e.g. set tags:
"${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME
}}:${{ env.GCR_VERSION }},${{ env.CONTAINER_REPO }}/${{ github.repository_owner
}}/${{ env.CONTAINER_NAME }}:latest" on one line) ensuring no YAML folding or
extra line breaks are used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your suggested fix makes it harder to read the source code.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🌐 Web query:

docker build-push-action v6 tags input format YAML

💡 Result:

Short answer: the v6 action accepts tags as either a comma-separated list or a YAML multiline string (pipe |) with one tag per line — and it also accepts the metadata-action output (steps.meta.outputs.tags). Examples:

  • comma-separated:
    tags: "v1,latest,sha-abc123". [1][2]

  • YAML multiline (recommended for readability / expressions):
    tags: |
    ghcr.io/my-org/my-app:latest
    ghcr.io/my-org/my-app:${{ github.ref_name }}. [3][4]

  • using docker/metadata-action output:
    tags: ${{ steps.meta.outputs.tags }} (metadata-action produces the tags string that build-push-action consumes). [4][5]

Sources: Docker Build & Push Action docs / GitHub Marketplace and Docker docs on GitHub Actions (examples above). [1][2][3][4][5]

References
[1] Docker Build & Push Action (Marketplace inputs table).
[2] docker/build-push-action GitHub repo (examples).
[3] Docker docs — Manage tags and labels with GitHub Actions (multiline examples).
[4] Docker docs — GitHub Actions and Docker (using steps.meta.outputs.tags).
[5] docker/metadata-action README (outputs.tags usage).


@networkfusion, You're absolutely right to prioritize readability. The folded scalar (>) format you're using is valid for docker/build-push-action@v6.

If you'd like an even more readable option, the Docker documentation actually recommends using the pipe (|) format with one tag per line:

        tags: |
          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:${{ env.GCR_VERSION }}
          ${{ env.CONTAINER_REPO }}/${{ github.repository_owner }}/${{ env.CONTAINER_NAME }}:latest

This format is clearer than the folded scalar and avoids the trailing comma. However, your current format works correctly, so this is purely a readability suggestion—no change is required.


✏️ Learnings added
Learnt from: networkfusion
Repo: nanoframework/nf-interpreter PR: 3198
File: .github/workflows/devcontainer-esp32.yml:62-64
Timestamp: 2025-11-05T16:43:42.592Z
Learning: In the nanoframework/nf-interpreter repository, the team prefers YAML folded scalar (>) or pipe (|) format for docker/build-push-action tags input over single-line comma-separated format for better readability in GitHub Actions workflows.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the original usage was correct?!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@networkfusion, Yes, you're absolutely right—the original pipe (|) format was correct for docker/build-push-action@v6. I apologize for the initial incorrect flag suggesting it was incompatible.

To clarify the changes in this PR:

  • Original format: Used pipe (|) with only the GCR_VERSION tag
  • New format: Uses folded scalar (>) with both the GCR_VERSION tag AND a new "latest" tag

Both the pipe (|) and folded scalar (>) formats are valid and work correctly with docker/build-push-action@v6. The substantive change here is the addition of the "latest" tag, not the format itself. The format change from | to > is inconsequential—either works fine.

Sorry for the confusion caused by the initial review comment.


🧠 Learnings used
Learnt from: networkfusion
Repo: nanoframework/nf-interpreter PR: 3198
File: .github/workflows/devcontainer-esp32.yml:62-64
Timestamp: 2025-11-05T16:43:42.592Z
Learning: In the nanoframework/nf-interpreter repository, the team prefers YAML folded scalar (>) or pipe (|) format for docker/build-push-action tags input over single-line comma-separated format for better readability in GitHub Actions workflows.

Learnt from: josesimoes
Repo: nanoframework/nf-interpreter PR: 3144
File: azure-pipelines-templates/download-install-cmake.yml:1-3
Timestamp: 2025-04-07T14:39:17.549Z
Learning: In the nanoframework/nf-interpreter repository, Unix-style line endings (\n) are not mandatory, even for YAML files, despite static analysis tools like YAMLlint flagging them as errors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are wrong. The original implementation included 2 tags (including latest).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@networkfusion, You're absolutely correct, and I apologize for the error. Looking at the diff more carefully, the :latest tag was indeed present in the original implementation. The line without a - or + prefix indicates it existed in both versions.

The actual change in this PR is:

  • Original format: Used pipe (|) with both the GCR_VERSION tag AND the latest tag (two separate lines)
  • New format: Uses folded scalar (>) with the same two tags, but with a comma separator

Both the original and new formats are valid for docker/build-push-action@v6. The change is purely in the YAML formatting approach, not in which tags are being pushed.

Thank you for the correction, and I apologize for the confusion in my previous responses.


🧠 Learnings used
Learnt from: networkfusion
Repo: nanoframework/nf-interpreter PR: 3198
File: .github/workflows/devcontainer-esp32.yml:62-64
Timestamp: 2025-11-05T16:43:42.592Z
Learning: In the nanoframework/nf-interpreter repository, the team prefers YAML folded scalar (>) or pipe (|) format for docker/build-push-action tags input over single-line comma-separated format for better readability in GitHub Actions workflows.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants